-
-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement filtering for imported data #4118
Conversation
c4ac50a
to
806c390
Compare
c9273ea
to
849315a
Compare
|> do_decide_table() | ||
end | ||
|
||
defp transform_filters(query) do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Q: Can you explain the business logic here?
I'm planning on moving business logic soonish so would need to understand this better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The transform_filters
function is used for:
- dropping the redundant "is name pageview" filter which doesn't make sense it imported data
- mapping an
event:goal
filter into eitherevent:name
orevent:page
In the imported querying logic we need to map goals to name/page in two places:
- deciding whether imported data can be included or not (i.e. schema_supports_query)
- actually applying the filter
The general business logic with imported data is that we can query different properties in relation to each other (breakdown prop / filters) as long as they belong to the same imported_*
table.
For example, when filtering by event:page==/blog
and event:goal==Visit /blog
, the query is supported and the data should be fetched from the imported_pages
table.
Happy to clarify it more in depth if you have any more specific questions :)
Request: If we can merge this faster by splitting this into separate FE and BE PRs I'd appreciate that. Blocked on a large chunk of query/APIv2 work on these changes. |
I think we don't need to split it and can go ahead and merge it on Monday. |
69dfcaa
to
efcb7ea
Compare
+ tests for pages, entry_pages, exit_pages and common filter types
efcb7ea
to
4e932dd
Compare
Changes
This PR adds support for filtering by imported data. What we can or cannot filter by is limited by the imported schemas. Usually that means that the relation between different properties does not exist - e.g. we cannot filter by page and break down by source. That information doesn't exist. However, in some tables, there are several properties - e.g.
imported_locations
containscountry
,region
andcity
propertis. That means that filtering bycountry
and breaking down bycity
is possible.Whenever filters are applied and imported data is included in the currently viewed period, we will display warning bubbles if a certain report is not able to include imported data:
Note that this does not mean that native data is excluded - the dashboard in the screenshot simply doesn't have any.
Tests
Changelog
Documentation
Dark mode